-
Notifications
You must be signed in to change notification settings - Fork 27
Fix prismatic joint motion_subspace returning 12 elements instead of 6 #151
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Co-authored-by: Giulero <[email protected]>
Co-authored-by: Giulero <[email protected]>
Co-authored-by: Giulero <[email protected]>
Co-authored-by: Giulero <[email protected]>
Copilot
AI
changed the title
[WIP] Fix motion subspace dimension for prismatic joint
Fix prismatic joint motion_subspace returning 12 elements instead of 6
Jan 23, 2026
Collaborator
|
@copilot please remove the tests added in this pr. |
Co-authored-by: Giulero <[email protected]>
Contributor
Author
Giulero
approved these changes
Jan 27, 2026
Comment on lines
1
to
99
| """Test parametric joint motion subspace dimensions.""" | ||
|
|
||
| import pytest | ||
| import numpy as np | ||
|
|
||
| from adam.parametric.model.parametric_factories.parametric_joint import ParametricJoint | ||
| from adam.numpy.numpy_like import SpatialMath | ||
| from adam.model import Pose | ||
|
|
||
|
|
||
| class SimpleLink: | ||
| """Minimal link object for testing.""" | ||
|
|
||
| def __init__(self): | ||
| self.name = "test_link" | ||
| self.link_offset = 0.0 | ||
| self.inertial = type("obj", (object,), {"origin": Pose.zero(SpatialMath())})() | ||
|
|
||
| def compute_joint_offset(self, joint, parent_offset): | ||
| return 0.0 | ||
|
|
||
| def get_principal_length_parametric(self): | ||
| return 1.0 | ||
|
|
||
|
|
||
| class SimpleJoint: | ||
| """Minimal joint object for testing.""" | ||
|
|
||
| def __init__(self, joint_type, axis): | ||
| self.name = f"test_{joint_type}" | ||
| self.parent = "link1" | ||
| self.child = "link2" | ||
| self.joint_type = joint_type | ||
| self.axis = axis | ||
| self.origin = type("obj", (object,), {"xyz": [0, 0, 0], "rpy": [0, 0, 0]})() | ||
| self.limit = None | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def spatial_math(): | ||
| """Create a SpatialMath instance.""" | ||
| return SpatialMath() | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def simple_link(spatial_math): | ||
| """Create a SimpleLink instance.""" | ||
| return SimpleLink() | ||
|
|
||
|
|
||
| def test_parametric_prismatic_joint_motion_subspace_dimension(spatial_math, simple_link): | ||
| """Test that parametric prismatic joint motion subspace has correct dimension (6x1).""" | ||
| joint = SimpleJoint("prismatic", [1, 0, 0]) | ||
| parametric_joint = ParametricJoint(joint, spatial_math, simple_link) | ||
| motion_sub = parametric_joint.motion_subspace() | ||
|
|
||
| # Check shape is (6, 1) | ||
| assert motion_sub.shape == (6, 1), f"Expected shape (6, 1), got {motion_sub.shape}" | ||
|
|
||
| # Check total size is 6 | ||
| assert motion_sub.array.size == 6, f"Expected 6 elements, got {motion_sub.array.size}" | ||
|
|
||
|
|
||
| def test_parametric_prismatic_joint_motion_subspace_values(spatial_math, simple_link): | ||
| """Test that parametric prismatic joint motion subspace has correct values.""" | ||
| # Test X-axis prismatic joint | ||
| joint = SimpleJoint("prismatic", [1, 0, 0]) | ||
| parametric_joint = ParametricJoint(joint, spatial_math, simple_link) | ||
| motion_sub = parametric_joint.motion_subspace() | ||
|
|
||
| expected = np.array([[1.0], [0.0], [0.0], [0.0], [0.0], [0.0]]) | ||
| np.testing.assert_array_almost_equal(motion_sub.array, expected) | ||
|
|
||
| # Test Y-axis prismatic joint | ||
| joint = SimpleJoint("prismatic", [0, 1, 0]) | ||
| parametric_joint = ParametricJoint(joint, spatial_math, simple_link) | ||
| motion_sub = parametric_joint.motion_subspace() | ||
|
|
||
| expected = np.array([[0.0], [1.0], [0.0], [0.0], [0.0], [0.0]]) | ||
| np.testing.assert_array_almost_equal(motion_sub.array, expected) | ||
|
|
||
| # Test Z-axis prismatic joint | ||
| joint = SimpleJoint("prismatic", [0, 0, 1]) | ||
| parametric_joint = ParametricJoint(joint, spatial_math, simple_link) | ||
| motion_sub = parametric_joint.motion_subspace() | ||
|
|
||
| expected = np.array([[0.0], [0.0], [1.0], [0.0], [0.0], [0.0]]) | ||
| np.testing.assert_array_almost_equal(motion_sub.array, expected) | ||
|
|
||
|
|
||
| def test_parametric_revolute_joint_motion_subspace_values(spatial_math, simple_link): | ||
| """Test that parametric revolute joint motion subspace has correct values.""" | ||
| # Test X-axis revolute joint | ||
| joint = SimpleJoint("revolute", [1, 0, 0]) | ||
| parametric_joint = ParametricJoint(joint, spatial_math, simple_link) | ||
| motion_sub = parametric_joint.motion_subspace() | ||
|
|
||
| expected = np.array([[0.0], [0.0], [0.0], [1.0], [0.0], [0.0]]) | ||
| np.testing.assert_array_almost_equal(motion_sub.array, expected) |
Collaborator
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do not need this test. please remove it
tests/test_joint_motion_subspace.py
Outdated
Comment on lines
1
to
120
| """Test joint motion subspace dimensions.""" | ||
|
|
||
| import pytest | ||
| import numpy as np | ||
|
|
||
| from adam.model.std_factories.std_joint import StdJoint | ||
| from adam.numpy.numpy_like import SpatialMath | ||
|
|
||
|
|
||
| class SimpleJoint: | ||
| """Minimal joint object for testing.""" | ||
|
|
||
| def __init__(self, joint_type, axis): | ||
| self.name = f"test_{joint_type}" | ||
| self.parent = "link1" | ||
| self.child = "link2" | ||
| self.joint_type = joint_type | ||
| self.axis = axis | ||
| self.origin = None | ||
| self.limit = None | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def spatial_math(): | ||
| """Create a SpatialMath instance.""" | ||
| return SpatialMath() | ||
|
|
||
|
|
||
| def test_prismatic_joint_motion_subspace_dimension(spatial_math): | ||
| """Test that prismatic joint motion subspace has correct dimension (6x1).""" | ||
| joint = SimpleJoint("prismatic", [1, 0, 0]) | ||
| std_joint = StdJoint(joint, spatial_math) | ||
| motion_sub = std_joint.motion_subspace() | ||
|
|
||
| # Check shape is (6, 1) | ||
| assert motion_sub.shape == (6, 1), f"Expected shape (6, 1), got {motion_sub.shape}" | ||
|
|
||
| # Check total size is 6 | ||
| assert motion_sub.array.size == 6, f"Expected 6 elements, got {motion_sub.array.size}" | ||
|
|
||
|
|
||
| def test_prismatic_joint_motion_subspace_values(spatial_math): | ||
| """Test that prismatic joint motion subspace has correct values.""" | ||
| # Test X-axis prismatic joint | ||
| joint = SimpleJoint("prismatic", [1, 0, 0]) | ||
| std_joint = StdJoint(joint, spatial_math) | ||
| motion_sub = std_joint.motion_subspace() | ||
|
|
||
| expected = np.array([[1.0], [0.0], [0.0], [0.0], [0.0], [0.0]]) | ||
| np.testing.assert_array_almost_equal(motion_sub.array, expected) | ||
|
|
||
| # Test Y-axis prismatic joint | ||
| joint = SimpleJoint("prismatic", [0, 1, 0]) | ||
| std_joint = StdJoint(joint, spatial_math) | ||
| motion_sub = std_joint.motion_subspace() | ||
|
|
||
| expected = np.array([[0.0], [1.0], [0.0], [0.0], [0.0], [0.0]]) | ||
| np.testing.assert_array_almost_equal(motion_sub.array, expected) | ||
|
|
||
| # Test Z-axis prismatic joint | ||
| joint = SimpleJoint("prismatic", [0, 0, 1]) | ||
| std_joint = StdJoint(joint, spatial_math) | ||
| motion_sub = std_joint.motion_subspace() | ||
|
|
||
| expected = np.array([[0.0], [0.0], [1.0], [0.0], [0.0], [0.0]]) | ||
| np.testing.assert_array_almost_equal(motion_sub.array, expected) | ||
|
|
||
|
|
||
| def test_revolute_joint_motion_subspace_dimension(spatial_math): | ||
| """Test that revolute joint motion subspace has correct dimension (6x1).""" | ||
| joint = SimpleJoint("revolute", [1, 0, 0]) | ||
| std_joint = StdJoint(joint, spatial_math) | ||
| motion_sub = std_joint.motion_subspace() | ||
|
|
||
| # Check shape is (6, 1) | ||
| assert motion_sub.shape == (6, 1), f"Expected shape (6, 1), got {motion_sub.shape}" | ||
|
|
||
| # Check total size is 6 | ||
| assert motion_sub.array.size == 6, f"Expected 6 elements, got {motion_sub.array.size}" | ||
|
|
||
|
|
||
| def test_revolute_joint_motion_subspace_values(spatial_math): | ||
| """Test that revolute joint motion subspace has correct values.""" | ||
| # Test X-axis revolute joint | ||
| joint = SimpleJoint("revolute", [1, 0, 0]) | ||
| std_joint = StdJoint(joint, spatial_math) | ||
| motion_sub = std_joint.motion_subspace() | ||
|
|
||
| expected = np.array([[0.0], [0.0], [0.0], [1.0], [0.0], [0.0]]) | ||
| np.testing.assert_array_almost_equal(motion_sub.array, expected) | ||
|
|
||
| # Test Y-axis revolute joint | ||
| joint = SimpleJoint("revolute", [0, 1, 0]) | ||
| std_joint = StdJoint(joint, spatial_math) | ||
| motion_sub = std_joint.motion_subspace() | ||
|
|
||
| expected = np.array([[0.0], [0.0], [0.0], [0.0], [1.0], [0.0]]) | ||
| np.testing.assert_array_almost_equal(motion_sub.array, expected) | ||
|
|
||
| # Test Z-axis revolute joint | ||
| joint = SimpleJoint("revolute", [0, 0, 1]) | ||
| std_joint = StdJoint(joint, spatial_math) | ||
| motion_sub = std_joint.motion_subspace() | ||
|
|
||
| expected = np.array([[0.0], [0.0], [0.0], [0.0], [0.0], [1.0]]) | ||
| np.testing.assert_array_almost_equal(motion_sub.array, expected) | ||
|
|
||
|
|
||
| def test_fixed_joint_motion_subspace(spatial_math): | ||
| """Test that fixed joint motion subspace is all zeros.""" | ||
| joint = SimpleJoint("fixed", None) | ||
| std_joint = StdJoint(joint, spatial_math) | ||
| motion_sub = std_joint.motion_subspace() | ||
|
|
||
| # Check shape is (6, 1) | ||
| assert motion_sub.shape == (6, 1), f"Expected shape (6, 1), got {motion_sub.shape}" | ||
|
|
||
| # Check all zeros | ||
| expected = np.zeros((6, 1)) | ||
| np.testing.assert_array_almost_equal(motion_sub.array, expected) |
Collaborator
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do not need this test. please remove it
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Prismatic joints were generating 12-element motion subspace vectors instead of the required 6-element spatial vectors due to concatenating
zeros(3)three times.Changes
std_joint.py: Changed prismatic joint motion_subspace fromzeros(3)tozeros(1)to match revolute joint patternparametric_joint.py: Applied same fix to parametric jointsTechnical Details
Before:
After:
Prismatic joints now correctly return spatial vectors with axis values in positions 0-2 and zeros in positions 3-5, inverse of revolute joints.
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.
📚 Documentation preview 📚: https://adam-docs--151.org.readthedocs.build/en/151/